Pull request overview

This PR introduces initial Language Server Protocol (LSP) scaffolding for Pyret by adding a VS Code node extension entrypoint that starts a language client, plus a TypeScript LSP server that forwards textDocument/definition (jump-to-def) requests to the existing Pyret compiler server, backed by a new in-memory compilation cache.

Changes:

• Add a Node-targeted VS Code extension entry (src/extension.ts) and webpack build config alongside the existing web extension.
• Add a new lsp/ TypeScript server project (including a temporary node server implementation) and wire it into the VS Code extension via vscode-languageclient.
• Refactor the Pyret compiler server protocol to support “info” requests and add an in-memory cache manager + LSP-side “jump-to-def” implementation.

Reviewed changes

Copilot reviewed 41 out of 48 changed files in this pull request and generated 7 comments.

default-start-context = {
  cache-manager: CS.default-compile-options.cache-manager,
  current-load-path: Filesystem.resolve("./"),
  cache-base-dir: Filesystem.resolve("./compiled"),
  compiled-read-only-dirs: empty,
  url-file-mode: CS.all-remote
}

default-test-context = {
  cache-manager: CS.default-compile-options.cache-manager,
  current-load-path: Filesystem.resolve("./"),
  cache-base-dir: Filesystem.resolve("./tests/compiled"),
  compiled-read-only-dirs: empty,
  url-file-mode: CS.all-remote

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Looking great! Mostly documentation nits, and a few opportunities for cleaner code.

Ultra-Review: PR #51 — Initial LSP

+2197 / -575 across 46 files. Core changes: (1) add Loc to Name AST variants, (2) Pyret-side LSP query infrastructure, (3) TS LSP server + VS Code node extension.

CRITICAL (4)

C1. server.js:145 — Unguarded JSON.parse crashes the server on malformed WebSocket input

No try/catch around JSON.parse(message). A single bad message from any client takes down the entire Pyret compilation server.

C2. server.js:141 — message is a Buffer in ws 8.x, not string

The @type {string} JSDoc is wrong. ws 8.x delivers Buffer by default. Works by accident (Node's JSON.parse accepts Buffer), but any typeof check will fail.

C3. server-node-tmp.ts:57 — execArgv: ["-max-old-space-size=8192"] needs double dash

Node V8 flags require --max-old-space-size=8192. Single-dash is silently ignored, so the 8GB heap limit is not applied.

C4. types.d.ts:13-19 — pyret_pos missing endCol field

The runtime reads p.endCol (ffi.js:51) but the type has no endCol. Code using this type will produce undefined for endCol, causing NaN in source locations.

MAJOR (14)

M1. server.js:119-186 — No 'error' handler on WebSocket connections

In Node, an unhandled 'error' event crashes the process. A client disconnect (TCP reset) will emit an error event, taking down the server.

M2. server.js:58-87 — Queued items hold references to closed WebSocket connections

No connection.readyState check before connection.send(). If a client disconnects while its request is queued, send() throws.

M3. server-node-tmp.ts:150-206 — WebSocket connections leak (never closed)

sendJumpToDefRequest opens a new WebSocket per request but never calls client.close() after resolving/rejecting.

M4. server-node-tmp.ts:150-206 — No timeout on WebSocket requests

If the Pyret server hangs, the promise never settles and the LSP client freezes indefinitely.

M5. server-node-tmp.ts:176 — Unguarded JSON.parse in WebSocket message handler

Same issue as C1, but on the LSP server side. Malformed data from the Pyret server crashes the LSP process.

M6. server-node-tmp.ts:112-119 — Stale socket file causes silent failure

If a previous server crashed without cleanup, the socket file exists but no server is listening. The code skips startPyretServer and all subsequent requests fail. No liveness check.

M7. server.arr:140-145 — ask block with no otherwise fallback

Only "jump-to-def" is handled. An unknown query string throws an exception instead of returning a clean error. Same issue at lines 160-178 for serialization.

M8. query.arr — All failure paths return E.left([list:]) with no diagnostic info

Every none branch returns an empty error list. The caller logs torepr(errors) which is always "[list:]". Makes debugging impossible.

M9. compile-lib.arr:431 — Query path skips canonicalize-provides

When options.query is true, provides aren't canonicalized (unlike the normal path at line 439). Downstream code expecting module-uri(...) NameOrigins may break.

M10. extension.ts:22-24 — serverModule path resolves outside the extension directory

context.asAbsolutePath(path.join("..", "lsp", "out", "server-node-tmp.js")) escapes the extension root. Works in dev, breaks in any packaged/installed extension.

M11. vscode/package.json:18-23 — files excludes dist/extension.js

The files allowlist only has dist/web/**/*. The node entry point won't be in the packaged extension.

M12. types.d.ts:189 — PFunction.name typed as number, should be string

Runtime sets this.name = name || "anonymous". Constructor param is string but the property is declared number.

M13. types.d.ts:334 — makeOpqaue typo (should be makeOpaque)

Will never match the actual runtime export. Also missing the equals parameter.

M14. types.d.ts:265-268 — runThunk generic signature doesn't match runtime

Callback receives SuccessResult | FailureResult, not T. The type parameter U is declared but unused.

MINOR (14)

• m1. server-node-tmp.ts:41-86 — startPyretServer promise can settle twice. No guard flag on "error" / "exit" handlers.
• m2. server-node-tmp.ts:66 — child.unref() + disconnect() means exit handler never fires. pyretServerProcess stays non-null forever.
• m3. cli-module-loader.arr:48-59 — CacheManager type incomplete. Missing get-cached-worklist, set-cached-worklist, invalidate. File-based cache doesn't implement these.
• m4. compile-structs.arr:2986-2991 — Default cache-manager stub only has 4 methods. Any code hitting other methods crashes.
• m5. server.js:92-98 — No error handler on HTTP server listen. process.send({type: 'success'}) fires before the server is actually listening.
• m6. server.js:97 — Double timestamp in log messages. makeLogger already prepends new Date().
• m7. vscode/.vscode/launch.json:10 — outFiles points to out/ but webpack outputs to dist/. Breakpoints won't bind.
• m8. vscode/package.json:126 — pyret.trace.server defaults to "verbose". Will flood the output channel for all users.
• m9. lsp/package.json:14 — @types/ws in dependencies instead of devDependencies.
• m10. lsp/package.json:7 — main: "index.js" points to nonexistent file.
• m11. ast-util.arr:1551-1601 — find-name-key-by-srcloc missing s-type-global / s-module-global handlers. Jump-to-def silently fails for type names and module names.
• m12. extension.ts:58 — client.start() not awaited/caught. Unhandled rejection if server fails to start.
• m13. types.d.ts:347-364 — check* functions accept unknown, not the specific JS types declared. These are runtime assertion functions on Pyret values.
• m14. types.d.ts:180 — POpaque missing val and equals fields.

NITS (6)

• ast.arr — Multiple # ZACK: comments should be cleaned up or formalized.
• server.arr:43-46 — Commented-out print statements.
• server-node-tmp.ts — File named -tmp but is the actual entry point.
• server-node.ts — Dead code (nothing references it).
• lsp/package.json:5 — "license": "UNLICENSED" on an open-source repo.
• server.js:169 — Stray semicolon after case block brace.

Pre-existing bugs found (not blocking, but worth tracking)

• ast-util.arr:1106 — t-ref(ann-to-typ(ann), false) missing l argument (needs 3 args).
• ast-util.arr:271 — s-method reconstructed with wrong arity (drops name and blocky).

Name-carries-Loc consistency: PASS

All pattern matches on s-global, s-module-global, s-type-global, s-atom across resolve-scope, type-check, type-structs, type-defaults, desugar, desugar-post-tc, anf, anf-loop-compiler, and ast-anf have been updated correctly. Name._equals compares by .key() only (excludes loc), so == comparisons with dummy-loc are safe.

Top 5 things to fix before merge

1. C1+M1 — Add error handling to server.js (try/catch JSON.parse, add ws error handlers). A single bad message shouldn't kill the compilation server.
2. C3 — Double-dash on --max-old-space-size=8192.
3. M3+M4 — Close WebSocket connections after use and add a timeout in sendJumpToDefRequest.
4. M8 — Add diagnostic strings to query.arr failure paths so debugging is possible.
5. M10+M11 — Fix extension packaging (files allowlist, serverModule path) before it bites someone.

Generated by Claude Code (claude-opus-4-6)